From e3ec96309bd2c56e87505738b77040c575954f16 Mon Sep 17 00:00:00 2001 From: "kfraser@localhost.localdomain" Date: Mon, 25 Sep 2006 10:22:17 +0100 Subject: [PATCH] [HVM] Use AMD's isntruction-length decoder for VMX as well as SVM MMIO decode. The VMX-defined instruction-length info field is not valid for use during most page faults. Hence we have to obtain the instruction length the slow-and-stupid way. This *will* go away when we throw away the wretched MMIO emulator. Signed-off-by: Keir Fraser --- xen/arch/x86/hvm/Makefile | 1 + xen/arch/x86/hvm/{svm => }/instrlen.c | 41 ++++++++++------------- xen/arch/x86/hvm/platform.c | 48 ++++++++++++++------------- xen/arch/x86/hvm/svm/Makefile | 1 - xen/arch/x86/hvm/svm/svm.c | 25 ++++++-------- xen/arch/x86/hvm/vmx/vmx.c | 40 ++++++++++++++++------ xen/include/asm-x86/hvm/hvm.h | 18 ++++------ xen/include/asm-x86/hvm/vmx/vmx.h | 26 --------------- 8 files changed, 91 insertions(+), 109 deletions(-) rename xen/arch/x86/hvm/{svm => }/instrlen.c (92%) diff --git a/xen/arch/x86/hvm/Makefile b/xen/arch/x86/hvm/Makefile index bc2cc1bf18..0f6e80f644 100644 --- a/xen/arch/x86/hvm/Makefile +++ b/xen/arch/x86/hvm/Makefile @@ -4,6 +4,7 @@ subdir-y += vmx obj-y += hvm.o obj-y += i8254.o obj-y += i8259.o +obj-y += instrlen.o obj-y += intercept.o obj-y += io.o obj-y += platform.o diff --git a/xen/arch/x86/hvm/svm/instrlen.c b/xen/arch/x86/hvm/instrlen.c similarity index 92% rename from xen/arch/x86/hvm/svm/instrlen.c rename to xen/arch/x86/hvm/instrlen.c index f7c78d0461..02bc9032a0 100644 --- a/xen/arch/x86/hvm/svm/instrlen.c +++ b/xen/arch/x86/hvm/instrlen.c @@ -10,25 +10,22 @@ */ /* - * TODO: the way in which we use svm_instrlen is very inefficient as is now - * stands. It will be worth while to return the actual instruction buffer + * TODO: The way in which we use hvm_instruction_length is very inefficient as + * it now stands. It will be worthwhile to return the actual instruction buffer * along with the instruction length since one of the reasons we are getting * the instruction length is to know how many instruction bytes we need to * fetch. */ #include -#include -#include +#include #include #include -#define DPRINTF DPRINTK #include /* read from guest memory */ extern int inst_copy_from_guest(unsigned char *buf, unsigned long eip, int length); -extern void svm_dump_inst(unsigned long eip); /* * Opcode effective-address decode tables. @@ -203,27 +200,26 @@ static uint8_t twobyte_table[256] = { * @_type: u8, u16, u32, s8, s16, or s32 * @_size: 1, 2, or 4 bytes * @_eip: address to fetch from guest memory - * @_length: updated! increments the current instruction length counter by _size + * @_length: increments the current instruction length counter by _size * - * INTERNAL this is used internally by svm_instrlen to fetch the next byte, + * This is used internally by hvm_instruction_length to fetch the next byte, * word, or dword from guest memory at location _eip. we currently use a local * unsigned long as the storage buffer since the most bytes we're gonna get * is limited to 4. */ -#define insn_fetch(_type, _size, _eip, _length) \ -({ unsigned long _x; \ - if ((rc = inst_copy_from_guest((unsigned char *)(&(_x)), \ - (unsigned long)(_eip), _size)) \ - != _size) \ - goto done; \ - (_eip) += (_size); \ - (_length) += (_size); \ - (_type)_x; \ +#define insn_fetch(_type, _size, _eip, _length) \ +({ unsigned long _x; \ + if ((rc = inst_copy_from_guest((unsigned char *)(&(_x)), \ + (unsigned long)(_eip), _size)) \ + != _size) \ + goto done; \ + (_eip) += (_size); \ + (_length) += (_size); \ + (_type)_x; \ }) - /** - * svn_instrlen - returns the current instructions length + * hvm_instruction_length - returns the current instructions length * * @regs: guest register state * @mode: guest operating mode @@ -231,7 +227,7 @@ static uint8_t twobyte_table[256] = { * EXTERNAL this routine calculates the length of the current instruction * pointed to by eip. The guest state is _not_ changed by this routine. */ -int svm_instrlen(struct cpu_user_regs *regs, int mode) +int hvm_instruction_length(struct cpu_user_regs *regs, int mode) { uint8_t b, d, twobyte = 0, rex_prefix = 0; uint8_t modrm, modrm_mod = 0, modrm_reg = 0, modrm_rm = 0; @@ -345,7 +341,7 @@ done_prefixes: if ( modrm_mod == 3 ) { - DPRINTF("Cannot parse ModRM.mod == 3.\n"); + DPRINTK("Cannot parse ModRM.mod == 3.\n"); goto cannot_emulate; } @@ -472,8 +468,7 @@ done: return length; cannot_emulate: - DPRINTF("Cannot emulate %02x at address %lx (eip %lx, mode %d)\n", + DPRINTK("Cannot emulate %02x at address %lx (eip %lx, mode %d)\n", b, (unsigned long)_regs.eip, (unsigned long)regs->eip, mode); - svm_dump_inst(_regs.eip); return -1; } diff --git a/xen/arch/x86/hvm/platform.c b/xen/arch/x86/hvm/platform.c index 8f42930e38..f84a41b06f 100644 --- a/xen/arch/x86/hvm/platform.c +++ b/xen/arch/x86/hvm/platform.c @@ -52,7 +52,7 @@ static inline long __get_reg_value(unsigned long reg, int size) case QUAD: return (long)(reg); default: - printf("Error: (__get_reg_value) Invalid reg size\n"); + printk("Error: (__get_reg_value) Invalid reg size\n"); domain_crash_synchronous(); } } @@ -78,7 +78,7 @@ long get_reg_value(int size, int index, int seg, struct cpu_user_regs *regs) case 7: /* %bh */ return (char)((regs->rbx & 0xFF00) >> 8); default: - printf("Error: (get_reg_value) Invalid index value\n"); + printk("Error: (get_reg_value) Invalid index value\n"); domain_crash_synchronous(); } /* NOTREACHED */ @@ -102,7 +102,7 @@ long get_reg_value(int size, int index, int seg, struct cpu_user_regs *regs) case 14: return __get_reg_value(regs->r14, size); case 15: return __get_reg_value(regs->r15, size); default: - printf("Error: (get_reg_value) Invalid index value\n"); + printk("Error: (get_reg_value) Invalid index value\n"); domain_crash_synchronous(); } } @@ -115,7 +115,7 @@ static inline long __get_reg_value(unsigned long reg, int size) case LONG: return (int)(reg & 0xFFFFFFFF); default: - printf("Error: (__get_reg_value) Invalid reg size\n"); + printk("Error: (__get_reg_value) Invalid reg size\n"); domain_crash_synchronous(); } } @@ -141,7 +141,7 @@ long get_reg_value(int size, int index, int seg, struct cpu_user_regs *regs) case 7: /* %bh */ return (char)((regs->ebx & 0xFF00) >> 8); default: - printf("Error: (get_reg_value) Invalid index value\n"); + printk("Error: (get_reg_value) Invalid index value\n"); domain_crash_synchronous(); } } @@ -156,7 +156,7 @@ long get_reg_value(int size, int index, int seg, struct cpu_user_regs *regs) case 6: return __get_reg_value(regs->esi, size); case 7: return __get_reg_value(regs->edi, size); default: - printf("Error: (get_reg_value) Invalid index value\n"); + printk("Error: (get_reg_value) Invalid index value\n"); domain_crash_synchronous(); } } @@ -464,7 +464,7 @@ static int hvm_decode(int realmode, unsigned char *opcode, struct instruction *i return DECODE_success; default: - printf("%x/%x, This opcode isn't handled yet!\n", + printk("%x/%x, This opcode isn't handled yet!\n", *opcode, ins_subtype); return DECODE_failure; } @@ -614,7 +614,7 @@ static int hvm_decode(int realmode, unsigned char *opcode, struct instruction *i break; default: - printf("%x, This opcode isn't handled yet!\n", *opcode); + printk("%x, This opcode isn't handled yet!\n", *opcode); return DECODE_failure; } @@ -675,12 +675,12 @@ static int hvm_decode(int realmode, unsigned char *opcode, struct instruction *i } else { - printf("0f %x, This opcode subtype isn't handled yet\n", *opcode); + printk("0f %x, This opcode subtype isn't handled yet\n", *opcode); return DECODE_failure; } default: - printf("0f %x, This opcode isn't handled yet\n", *opcode); + printk("0f %x, This opcode isn't handled yet\n", *opcode); return DECODE_failure; } } @@ -702,7 +702,7 @@ static void hvm_send_assist_req(struct vcpu *v) if ( unlikely(p->state != STATE_INVALID) ) { /* This indicates a bug in the device model. Crash the domain. */ - printf("Device model set bad IO state %d.\n", p->state); + printk("Device model set bad IO state %d.\n", p->state); domain_crash(v->domain); return; } @@ -733,7 +733,7 @@ void send_pio_req(struct cpu_user_regs *regs, unsigned long port, p = &vio->vp_ioreq; if ( p->state != STATE_INVALID ) - printf("WARNING: send pio with something already pending (%d)?\n", + printk("WARNING: send pio with something already pending (%d)?\n", p->state); p->dir = dir; p->pdata_valid = pvalid; @@ -776,14 +776,14 @@ void send_mmio_req( vio = get_vio(v->domain, v->vcpu_id); if (vio == NULL) { - printf("bad shared page\n"); + printk("bad shared page\n"); domain_crash_synchronous(); } p = &vio->vp_ioreq; if ( p->state != STATE_INVALID ) - printf("WARNING: send mmio with something already pending (%d)?\n", + printk("WARNING: send mmio with something already pending (%d)?\n", p->state); p->dir = dir; p->pdata_valid = pvalid; @@ -841,7 +841,7 @@ static void mmio_operands(int type, unsigned long gpa, struct instruction *inst, else send_mmio_req(type, gpa, 1, inst->op_size, 0, IOREQ_READ, 0); } else { - printf("mmio_operands: invalid operand\n"); + printk("mmio_operands: invalid operand\n"); domain_crash_synchronous(); } } @@ -866,8 +866,10 @@ void handle_mmio(unsigned long va, unsigned long gpa) memcpy(regs, guest_cpu_user_regs(), HVM_CONTEXT_STACK_BYTES); hvm_store_cpu_guest_regs(v, regs, NULL); - if ((inst_len = hvm_instruction_length(v)) <= 0) { - printf("handle_mmio: failed to get instruction length\n"); + inst_len = hvm_instruction_length(regs, hvm_guest_x86_mode(v)); + if ( inst_len <= 0 ) + { + printk("handle_mmio: failed to get instruction length\n"); domain_crash_synchronous(); } @@ -880,19 +882,19 @@ void handle_mmio(unsigned long va, unsigned long gpa) memset(inst, 0, MAX_INST_LEN); ret = inst_copy_from_guest(inst, inst_addr, inst_len); if (ret != inst_len) { - printf("handle_mmio: failed to copy instruction\n"); + printk("handle_mmio: failed to copy instruction\n"); domain_crash_synchronous(); } init_instruction(&mmio_inst); if (hvm_decode(realmode, inst, &mmio_inst) == DECODE_failure) { - printf("handle_mmio: failed to decode instruction\n"); - printf("mmio opcode: va 0x%lx, gpa 0x%lx, len %d:", + printk("handle_mmio: failed to decode instruction\n"); + printk("mmio opcode: va 0x%lx, gpa 0x%lx, len %d:", va, gpa, inst_len); for (i = 0; i < inst_len; i++) - printf(" %02x", inst[i] & 0xFF); - printf("\n"); + printk(" %02x", inst[i] & 0xFF); + printk("\n"); domain_crash_synchronous(); } @@ -1073,7 +1075,7 @@ void handle_mmio(unsigned long va, unsigned long gpa) break; default: - printf("Unhandled MMIO instruction\n"); + printk("Unhandled MMIO instruction\n"); domain_crash_synchronous(); } } diff --git a/xen/arch/x86/hvm/svm/Makefile b/xen/arch/x86/hvm/svm/Makefile index e3baf066a0..523fd0bd2a 100644 --- a/xen/arch/x86/hvm/svm/Makefile +++ b/xen/arch/x86/hvm/svm/Makefile @@ -2,7 +2,6 @@ subdir-$(x86_32) += x86_32 subdir-$(x86_64) += x86_64 obj-y += emulate.o -obj-y += instrlen.o obj-y += intr.o obj-y += svm.o obj-y += vmcb.o diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c index da0e1a9d2f..0a605e4407 100644 --- a/xen/arch/x86/hvm/svm/svm.c +++ b/xen/arch/x86/hvm/svm/svm.c @@ -44,6 +44,7 @@ #include #include #include +#include #include #define SVM_EXTRA_DEBUG @@ -60,7 +61,6 @@ extern int inst_copy_from_guest(unsigned char *buf, unsigned long guest_eip, extern asmlinkage void do_IRQ(struct cpu_user_regs *); extern void send_pio_req(struct cpu_user_regs *regs, unsigned long port, unsigned long count, int size, long value, int dir, int pvalid); -extern int svm_instrlen(struct cpu_user_regs *regs, int mode); extern void svm_dump_inst(unsigned long eip); extern int svm_dbg_on; void svm_dump_regs(const char *from, struct cpu_user_regs *regs); @@ -468,21 +468,19 @@ static int svm_realmode(struct vcpu *v) return (eflags & X86_EFLAGS_VM) || !(cr0 & X86_CR0_PE); } -int svm_guest_x86_mode(struct vcpu *v) +static int svm_guest_x86_mode(struct vcpu *v) { struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb; - unsigned long cr0 = vmcb->cr0, eflags = vmcb->rflags, mode; - /* check which operating mode the guest is running */ - if( vmcb->efer & EFER_LMA ) - mode = vmcb->cs.attributes.fields.l ? 8 : 4; - else - mode = (eflags & X86_EFLAGS_VM) || !(cr0 & X86_CR0_PE) ? 2 : 4; - return mode; -} -int svm_instruction_length(struct vcpu *v) -{ - return svm_instrlen(guest_cpu_user_regs(), svm_guest_x86_mode(v)); + if ( vmcb->efer & EFER_LMA ) + return (vmcb->cs.attributes.fields.l ? + X86EMUL_MODE_PROT64 : X86EMUL_MODE_PROT32); + + if ( svm_realmode(v) ) + return X86EMUL_MODE_REAL; + + return (vmcb->cs.attributes.fields.db ? + X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16); } void svm_update_host_cr3(struct vcpu *v) @@ -878,7 +876,6 @@ int start_svm(void) hvm_funcs.long_mode_enabled = svm_long_mode_enabled; hvm_funcs.pae_enabled = svm_pae_enabled; hvm_funcs.guest_x86_mode = svm_guest_x86_mode; - hvm_funcs.instruction_length = svm_instruction_length; hvm_funcs.get_guest_ctrl_reg = svm_get_ctrl_reg; hvm_funcs.update_host_cr3 = svm_update_host_cr3; diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index 3996b0f078..cda90bc76d 100644 --- a/xen/arch/x86/hvm/vmx/vmx.c +++ b/xen/arch/x86/hvm/vmx/vmx.c @@ -45,6 +45,7 @@ #include #include #include +#include extern uint32_t vlapic_update_ppr(struct vlapic *vlapic); @@ -593,15 +594,6 @@ static void vmx_load_cpu_guest_regs(struct vcpu *v, struct cpu_user_regs *regs) vmx_vmcs_exit(v); } -static int vmx_instruction_length(struct vcpu *v) -{ - unsigned long inst_len; - - if ( __vmread(VM_EXIT_INSTRUCTION_LEN, &inst_len) ) /* XXX Unsafe XXX */ - return 0; - return inst_len; -} - static unsigned long vmx_get_ctrl_reg(struct vcpu *v, unsigned int num) { switch ( num ) @@ -729,6 +721,35 @@ static void vmx_init_hypercall_page(struct domain *d, void *hypercall_page) *(u16 *)(hypercall_page + (__HYPERVISOR_iret * 32)) = 0x0b0f; /* ud2 */ } +static int vmx_realmode(struct vcpu *v) +{ + unsigned long rflags; + + ASSERT(v == current); + + __vmread(GUEST_RFLAGS, &rflags); + return rflags & X86_EFLAGS_VM; +} + +static int vmx_guest_x86_mode(struct vcpu *v) +{ + unsigned long cs_ar_bytes; + + ASSERT(v == current); + + __vmread(GUEST_CS_AR_BYTES, &cs_ar_bytes); + + if ( vmx_long_mode_enabled(v) ) + return ((cs_ar_bytes & (1u<<13)) ? + X86EMUL_MODE_PROT64 : X86EMUL_MODE_PROT32); + + if ( vmx_realmode(v) ) + return X86EMUL_MODE_REAL; + + return ((cs_ar_bytes & (1u<<14)) ? + X86EMUL_MODE_PROT32 : X86EMUL_MODE_PROT16); +} + /* Setup HVM interfaces */ static void vmx_setup_hvm_funcs(void) { @@ -748,7 +769,6 @@ static void vmx_setup_hvm_funcs(void) hvm_funcs.long_mode_enabled = vmx_long_mode_enabled; hvm_funcs.pae_enabled = vmx_pae_enabled; hvm_funcs.guest_x86_mode = vmx_guest_x86_mode; - hvm_funcs.instruction_length = vmx_instruction_length; hvm_funcs.get_guest_ctrl_reg = vmx_get_ctrl_reg; hvm_funcs.update_host_cr3 = vmx_update_host_cr3; diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 5f1fa73efe..bc950677c6 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -51,15 +51,13 @@ struct hvm_function_table { * Examine specifics of the guest state: * 1) determine whether the guest is in real or vm8086 mode, * 2) determine whether paging is enabled, - * 3) return the length of the instruction that caused an exit. - * 4) return the current guest control-register value + * 3) return the current guest control-register value */ int (*realmode)(struct vcpu *v); int (*paging_enabled)(struct vcpu *v); int (*long_mode_enabled)(struct vcpu *v); int (*pae_enabled)(struct vcpu *v); int (*guest_x86_mode)(struct vcpu *v); - int (*instruction_length)(struct vcpu *v); unsigned long (*get_guest_ctrl_reg)(struct vcpu *v, unsigned int num); /* @@ -159,11 +157,7 @@ hvm_guest_x86_mode(struct vcpu *v) return hvm_funcs.guest_x86_mode(v); } -static inline int -hvm_instruction_length(struct vcpu *v) -{ - return hvm_funcs.instruction_length(v); -} +int hvm_instruction_length(struct cpu_user_regs *regs, int mode); static inline void hvm_update_host_cr3(struct vcpu *v) @@ -182,9 +176,9 @@ hvm_get_guest_ctrl_reg(struct vcpu *v, unsigned int num) return 0; /* force to fail */ } -extern void hvm_stts(struct vcpu *v); -extern void hvm_set_guest_time(struct vcpu *v, u64 gtime); -extern void hvm_do_resume(struct vcpu *v); +void hvm_stts(struct vcpu *v); +void hvm_set_guest_time(struct vcpu *v, u64 gtime); +void hvm_do_resume(struct vcpu *v); static inline void hvm_init_ap_context(struct vcpu_guest_context *ctxt, @@ -193,6 +187,6 @@ hvm_init_ap_context(struct vcpu_guest_context *ctxt, return hvm_funcs.init_ap_context(ctxt, vcpuid, trampoline_vector); } -extern int hvm_bringup_ap(int vcpuid, int trampoline_vector); +int hvm_bringup_ap(int vcpuid, int trampoline_vector); #endif /* __ASM_X86_HVM_HVM_H__ */ diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-x86/hvm/vmx/vmx.h index a77b4d529a..fadb551e7c 100644 --- a/xen/include/asm-x86/hvm/vmx/vmx.h +++ b/xen/include/asm-x86/hvm/vmx/vmx.h @@ -424,16 +424,6 @@ static inline int vmx_pae_enabled(struct vcpu *v) return VMX_PAE_GUEST(current); } -/* Works only for vcpu == current */ -static inline int vmx_realmode(struct vcpu *v) -{ - unsigned long rflags; - ASSERT(v == current); - - __vmread(GUEST_RFLAGS, &rflags); - return rflags & X86_EFLAGS_VM; -} - /* Works only for vcpu == current */ static inline void vmx_update_host_cr3(struct vcpu *v) { @@ -441,22 +431,6 @@ static inline void vmx_update_host_cr3(struct vcpu *v) __vmwrite(HOST_CR3, v->arch.cr3); } -static inline int vmx_guest_x86_mode(struct vcpu *v) -{ - unsigned long cs_ar_bytes; - ASSERT(v == current); - - if ( vmx_long_mode_enabled(v) ) - { - __vmread(GUEST_CS_AR_BYTES, &cs_ar_bytes); - return (cs_ar_bytes & (1u<<13)) ? 8 : 4; - } - if ( vmx_realmode(v) ) - return 2; - __vmread(GUEST_CS_AR_BYTES, &cs_ar_bytes); - return (cs_ar_bytes & (1u<<14)) ? 4 : 2; -} - static inline int vmx_pgbit_test(struct vcpu *v) { unsigned long cr0; -- 2.30.2